gh-121645: Add PyBytes_Join() function#121646
Conversation
|
@serhiy-storchaka: Please review the updated PR. I tried to address most of your comments. |
|
@serhiy-storchaka: I addressed most of your comments, except of the one one the doc:
|
serhiy-storchaka
left a comment
There was a problem hiding this comment.
LGTM, but please discuss first the behavior for the NULL separator.
|
@encukou: I updated the PR to address your review. |
|
I created capi-workgroup/decisions#36 |
* Replace _PyBytes_Join() with PyBytes_Join(). * Keep _PyBytes_Join() as an alias to PyBytes_Join().
It was decided to reject NULL separator. I updated my PR to respect the C API Working Group decision. I also rebased the PR to fix merge conflicts. @erlend-aasland @encukou @serhiy-storchaka: Would you mind to review the updated PR? |
|
Please do not use rebase so later in the review. So I do not see difference with already reviewed code and need to re-read all from the start. Last two days I have power only for 2 intervals of 2 hours per day, so new review may take a time. |
Do you mean that rebasing on main is bad, or is squashing commits which is bad for review? Sorry, I will avoid squashing commits next time. |
|
Normally, I can find a link "changes since your last review". It is the best scenario. If you rebased without squashing, I can still manually select recent commits. This is less convenient, and there is no guarantee that you did not modify previous commits. If you squashed commits, all is lost. I do this only immediately after creating commit or pushing new changes, when there is a little chance that my previous change has been reviewed. |
encukou
left a comment
There was a problem hiding this comment.
LGTM!
I second Serhiy's request to not rebase CPython PRs.
|
Merged, thanks for reviews. |
📚 Documentation preview 📚: https://cpython-previews--121646.org.readthedocs.build/